JIT: Devirtualize shared generic virtual methods#123323
JIT: Devirtualize shared generic virtual methods#123323hez2010 wants to merge 39 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables JIT devirtualization for shared generic virtual methods (GVM) that don't require runtime lookups. Previously, all shared GVMs were blocked from devirtualization due to concerns about having the right generic context. This change unblocks devirtualization when the instantiating stub doesn't need a runtime lookup, by checking for the presence of a GT_RUNTIMELOOKUP node before proceeding.
Changes:
- Introduced
needsMethodContextflag to track when a method context is needed for devirtualization - For shared generic methods, obtain the instantiating stub and set
needsMethodContext = true - Unified handling of array interface and generic virtual method devirtualization paths
- Added runtime lookup check in the JIT to bail out when a lookup is needed but context is unavailable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/vm/jitinterface.cpp | Added logic to detect shared generic methods and obtain instantiating stubs, unified array and GVM devirtualization handling |
| src/coreclr/jit/importercalls.cpp | Updated assertions to allow GVM in AOT scenarios, added runtime lookup check to prevent devirtualization when context is unavailable |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Failures seem to be caused by missing context during spmi replay. Otherwise all tests are passing. |
bf5b09a to
c8b7f69
Compare
|
I realised that the existing runtime lookup tests are not actually doing runtime lookup, they are const lookup. |
src/coreclr/jit/importercalls.cpp
Outdated
| // If we don't know the array type exactly we may have the wrong interface type here. | ||
| // Bail out. | ||
| // | ||
| if (!isExact) |
There was a problem hiding this comment.
The needsCompileTimeLookup path unconditionally bails out when !isExact with an "Array interface devirt" message, but needsCompileTimeLookup can now also be true for shared generic virtual method devirtualization (not just array-interface devirt). This can prevent devirtualization in cases where objClassIsFinal/derivedMethodIsFinal would otherwise allow it. Consider gating the !isExact bailout (and the diagnostic text) to the array-interface scenario only, and allow the GVM path to proceed without requiring isExact.
| // If we don't know the array type exactly we may have the wrong interface type here. | |
| // Bail out. | |
| // | |
| if (!isExact) | |
| // For array interface devirtualization, if we don't know the array type exactly | |
| // we may have the wrong interface type here. Bail out. | |
| // | |
| if (!call->IsGenericVirtual(this) && !isExact) |
|
Test failures seem unrelated. |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| helperArg = new MethodWithToken(methodDesc, HandleToModuleToken(ref pResolvedToken), constrainedType, unboxing: false, context: sharedMethod); | ||
| if (helperArgToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_DevirtualizedMethod) | ||
| { | ||
| helperArg = ComputeMethodWithToken(HandleToObject(pResolvedToken.hMethod), ref helperArgToken, constrainedType, false); |
There was a problem hiding this comment.
In getReadyToRunHelper, when handling a generic handle lookup for a devirtualized method token (CORINFO_TOKENKIND_DevirtualizedMethod), the code calls ComputeMethodWithToken using HandleToObject(pResolvedToken.hMethod). At this point pResolvedToken is the token passed to the helper request (often the original/base token), while helperArgToken is the devirtualized token. This can cause the ReadyToRun helper to be constructed for the wrong method. Use the devirtualized method handle (e.g., methodDesc / helperArgToken.hMethod) when calling ComputeMethodWithToken so the fixup encodes the devirtualized target correctly.
| helperArg = ComputeMethodWithToken(HandleToObject(pResolvedToken.hMethod), ref helperArgToken, constrainedType, false); | |
| helperArg = ComputeMethodWithToken(methodDesc, ref helperArgToken, constrainedType, false); |
|
I think the pri1 failure should be addressed (it was crossgen2 related). @jakobbotsch I would like one more outerloop run. |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |

Previously in #122023, we hit an issue with GVM devirtualization when the devirtualized target is a shared generic method. GVM calls are imported with a runtime lookup that is specific to the base method. After devirtualization, the call requires the instantiation argument for the implementing method, and the existing lookup cannot be reused.
This PR unblocks devirtualization for shared generic targets by ensuring the call receives the correct instantiation parameter for the devirtualized method:
The multiple ad-hoc flags in
dvInfonow have been unified into a singleinstParamLookupWhen the target does not require a runtime lookup, we already know the exact generic context. We pass the instantiating stub as the inst param (shared with the existing array interface devirtualization path).
When the target requires a runtime lookup, we now introduced a new
DictionaryEntryKind::DevirtualizedMethodDescSlot, and pass it to theinstParamLookupso that later the VM knows that it needs to encode the class token from the devirtualized method instead of the original token.Also due to the
instParamLookupchange I implement the support for R2R as well.Example:
Codegen diff:
Contributes to #112596